config: Add a core/change-update-summary option

This commits adds and implements a boolean repo config option called
"change-update-summary" which updates the summary file every time a ref
changes (additions, updates, and deletions).

The main impetus for this feature is that the `ostree create-usb` and
`flatpak create-usb` commands depend on the repo summary being up to
date. On the command line you can work around this by asking the user to
run `ostree summary --update` but in the case of GNOME Software calling
out to `flatpak create-usb` this wouldn't work because it's running as a
user and the repo is owned by root. That strategy also means flatpak
can't update the repo metadata refs for fear of invalidating the
summary.

Another use case for this relates to LAN updates. Specifically, the
component of eos-updater that generates DNS-SD records advertising ostree
refs depends on the repo summary being up to date.

Since ostree_repo_regenerate_summary() now takes an exclusive lock, this
should be safe to enable. However it's not enabled by default because of
the performance cost, and because it's more useful on clients than
servers (which likely have another mechanism for updating the summary).

Fixes https://github.com/ostreedev/ostree/issues/1664

Closes: #1681
Approved by: jlebon

diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml
index 0d39f41ecf8c4e0042bb3689ba795362020d3f30..6149248fd449a046751ad404de107d2711675e81 100644 (file)
--- a/man/ostree.repo-config.xml
+++ b/man/ostree.repo-config.xml
@@ -93,6 +93,16 @@ Boston, MA 02111-1307, USA.
         to <literal>false</literal>.</para></listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><varname>change-update-summary</varname></term>
+        <listitem><para>Boolean value controlling whether or not to
+        automatically update the summary file after any ref is added,
+        removed, or updated. This covers a superset of the cases covered by
+        commit-update-summary, with the exception of orphan commits which
+        shouldn't affect the summary anyway. Defaults to <literal>false</literal>.
+        </para></listitem>
+      </varlistentry>
+
       <varlistentry>
         <term><varname>fsync</varname></term>
         <listitem><para>Boolean value controlling whether or not to

diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index 632e396c77dd885cbc137b7e95ca985b5cca07b7..12ee688821a77ea44ec5e140df09752b9523de46 100644 (file)
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -2202,6 +2202,12 @@ ostree_repo_commit_transaction (OstreeRepo                  *self,
       return FALSE;
   g_clear_pointer (&self->txn.collection_refs, g_hash_table_destroy);
 
+  /* Update the summary if change-update-summary is set, because doing so was
+   * delayed for each ref change during the transaction.
+   */
+  if (!_ostree_repo_maybe_regenerate_summary (self, cancellable, error))
+    return FALSE;
+
   self->in_transaction = FALSE;
 
   if (!ot_ensure_unlinked_at (self->repo_dir_fd, "transaction", 0))

diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h
index 0a44763472189b52d79f81186a61a568c9412c21..4e5043268552f39056c11ba4b4312723f7e7e811 100644 (file)
--- a/src/libostree/ostree-repo-private.h
+++ b/src/libostree/ostree-repo-private.h
@@ -437,6 +437,11 @@ _ostree_repo_get_remote_inherited (OstreeRepo  *self,
                                    const char  *name,
                                    GError     **error);
 
+gboolean
+_ostree_repo_maybe_regenerate_summary (OstreeRepo    *self,
+                                       GCancellable  *cancellable,
+                                       GError       **error);
+
 /* Locking APIs are currently private.
  * See https://github.com/ostreedev/ostree/pull/1555
  */

diff --git a/src/libostree/ostree-repo-refs.c b/src/libostree/ostree-repo-refs.c
index 2600cb7ce250b5fb70fe10bf0dddd35258d714e3..83d11c1b8053ace832e2d90797e0b2fbc474e556 100644 (file)
--- a/src/libostree/ostree-repo-refs.c
+++ b/src/libostree/ostree-repo-refs.c
@@ -1144,6 +1144,11 @@ _ostree_repo_write_ref (OstreeRepo                 *self,
   if (!_ostree_repo_update_mtime (self, error))
     return FALSE;
 
+  /* Update the summary after updating the mtime so the summary doesn't look
+   * out of date */
+  if (!self->in_transaction && !_ostree_repo_maybe_regenerate_summary (self, cancellable, error))
+    return FALSE;
+
   return TRUE;
 }
 

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 7acd094068d6aa349e418668a8373b7a9a784e62..10f87dba80550cbd9b77f02aa4bc488541191f1c 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -5386,7 +5386,8 @@ summary_add_ref_entry (OstreeRepo       *self,
  * will aid clients in working out when to check for updates.
  *
  * It is regenerated automatically after a commit if
- * `core/commit-update-summary` is set.
+ * `core/commit-update-summary` is set, and automatically after any ref is
+ * added, removed, or updated if `core/change-update-summary` is set.
  *
  * If the `core/collection-id` key is set in the configuration, it will be
  * included as %OSTREE_SUMMARY_COLLECTION_ID in the summary file. Refs that
@@ -5592,6 +5593,28 @@ ostree_repo_regenerate_summary (OstreeRepo     *self,
   return TRUE;
 }
 
+/* Regenerate the summary if `core/change-update-summary` is set */
+gboolean
+_ostree_repo_maybe_regenerate_summary (OstreeRepo    *self,
+                                       GCancellable  *cancellable,
+                                       GError       **error)
+{
+  gboolean update_summary;
+
+  if (!ot_keyfile_get_boolean_with_default (self->config, "core",
+                                            "change-update-summary", FALSE,
+                                            &update_summary, error))
+    return FALSE;
+
+  if (update_summary && !ostree_repo_regenerate_summary (self,
+                                                         NULL,
+                                                         cancellable,
+                                                         error))
+    return FALSE;
+
+  return TRUE;
+}
+
 gboolean
 _ostree_repo_is_locked_tmpdir (const char *filename)
 {

diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c
index c2f78700807e6b7c260086b3aef2ae0338d18b72..ded6522fc5fbe84989b800cf7def073d2aa580e2 100644 (file)
--- a/src/ostree/ot-builtin-commit.c
+++ b/src/ostree/ot-builtin-commit.c
@@ -752,8 +752,8 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio
 
   if (!skip_commit)
     {
-      gboolean update_summary;
       guint64 timestamp;
+      gboolean change_update_summary;
 
       if (!opt_no_bindings)
         {
@@ -824,21 +824,32 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio
       if (!ostree_repo_commit_transaction (repo, &stats, cancellable, error))
         goto out;
 
-      /* The default for this option is FALSE, even for archive repos,
-       * because ostree supports multiple processes committing to the same
-       * repo (but different refs) concurrently, and in fact gnome-continuous
-       * actually does this.  In that context it's best to update the summary
-       * explicitly instead of automatically here. */
       if (!ot_keyfile_get_boolean_with_default (ostree_repo_get_config (repo), "core",
-                                                "commit-update-summary", FALSE,
-                                                &update_summary, error))
+                                                "change-update-summary", FALSE,
+                                                &change_update_summary, error))
         goto out;
 
-      if (update_summary && !ostree_repo_regenerate_summary (repo,
-                                                             NULL,
-                                                             cancellable,
-                                                             error))
-        goto out;
+      /* No need to update it again if we did for each ref change */
+      if (opt_orphan || !change_update_summary)
+        {
+          gboolean commit_update_summary;
+
+          /* The default for this option is FALSE, even for archive repos,
+           * because ostree supports multiple processes committing to the same
+           * repo (but different refs) concurrently, and in fact gnome-continuous
+           * actually does this.  In that context it's best to update the summary
+           * explicitly instead of automatically here. */
+          if (!ot_keyfile_get_boolean_with_default (ostree_repo_get_config (repo), "core",
+                                                    "commit-update-summary", FALSE,
+                                                    &commit_update_summary, error))
+            goto out;
+
+          if (commit_update_summary && !ostree_repo_regenerate_summary (repo,
+                                                                        NULL,
+                                                                        cancellable,
+                                                                        error))
+            goto out;
+        }
     }
   else
     {

diff --git a/tests/test-auto-summary.sh b/tests/test-auto-summary.sh
index b12f1593b70aa7bc3476241fb814f2963dac0310..5811fcdec7bf8a1446fa4a523c374cabb6a91eb0 100755 (executable)
--- a/tests/test-auto-summary.sh
+++ b/tests/test-auto-summary.sh
@@ -29,6 +29,7 @@ echo "1..4"
 setup_test_repository "bare"
 echo "ok setup"
 
+# Check that without commit-update-summary set, creating a commit doesn't update the summary
 mkdir test
 
 echo hello > test/a
@@ -47,6 +48,7 @@ echo "ok commit 2"
 
 assert_streq "$OLD_MD5" "$(md5sum repo/summary)"
 
+# Check that with commit-update-summary set, creating a commit updates the summary
 $OSTREE --repo=repo config set core.commit-update-summary true
 
 echo hello3 > test/a
@@ -56,7 +58,46 @@ echo "ok commit 3"
 
 assert_not_streq "$OLD_MD5" "$(md5sum repo/summary)"
 
+$OSTREE --repo=repo config set core.commit-update-summary false
+
 # Check that summary --update deletes the .sig file
 touch repo/summary.sig
 $OSTREE summary --update
 assert_not_has_file repo/summary.sig
+
+# Check that without change-update-summary set, adding, changing, or deleting a ref doesn't update the summary
+$OSTREE summary --update
+OLD_MD5=$(md5sum repo/summary)
+$OSTREE commit -b test2 -s "A commit" test
+
+assert_streq "$OLD_MD5" "$(md5sum repo/summary)"
+
+$OSTREE reset test test^
+
+assert_streq "$OLD_MD5" "$(md5sum repo/summary)"
+
+$OSTREE refs --delete test
+
+assert_streq "$OLD_MD5" "$(md5sum repo/summary)"
+
+# Check that with change-update-summary set, adding, changing, or deleting a ref updates the summary
+$OSTREE --repo=repo config set core.change-update-summary true
+
+$OSTREE summary --update
+OLD_MD5=$(md5sum repo/summary)
+echo hello > test/a
+$OSTREE commit -b test -s "A commit" test
+echo hello2 > test/a
+$OSTREE commit -b test -s "Another commit" test
+
+assert_not_streq "$OLD_MD5" "$(md5sum repo/summary)"
+
+OLD_MD5=$(md5sum repo/summary)
+$OSTREE reset test test^
+
+assert_not_streq "$OLD_MD5" "$(md5sum repo/summary)"
+
+OLD_MD5=$(md5sum repo/summary)
+$OSTREE refs --delete test
+
+assert_not_streq "$OLD_MD5" "$(md5sum repo/summary)"